feat(cli): add option for a single output-dir and hide options#1099
feat(cli): add option for a single output-dir and hide options#1099jlarkin09 wants to merge 1 commit intopython-wheel-build:mainfrom
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change consolidates output directory handling in the CLI by introducing a new Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/fromager/__main__.py (1)
164-185: ⚡ Quick winAdd a docstring to
main()after expanding its public CLI contract.
main()gained new behavior and parameters (output_dir+ override precedence), so a short docstring here would improve maintainability and CLI intent clarity.As per coding guidelines, "Add docstrings on all public functions and classes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/fromager/__main__.py` around lines 164 - 185, Add a concise docstring to the public function main() explaining its purpose as the CLI entrypoint, summarizing the new behavior and parameter roles (notably output_dir and its override precedence relative to settings_file/settings_dir), documenting key parameters like output_dir, sdists_repo, wheels_repo, work_dir, patches_dir, settings_file/settings_dir, constraints_file, cleanup, variant, jobs, network_isolation, and min_release_age, and stating it returns None; place the docstring directly after the def main(...) signature so future maintainers understand the CLI contract and override rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/fromager/__main__.py`:
- Around line 164-185: Add a concise docstring to the public function main()
explaining its purpose as the CLI entrypoint, summarizing the new behavior and
parameter roles (notably output_dir and its override precedence relative to
settings_file/settings_dir), documenting key parameters like output_dir,
sdists_repo, wheels_repo, work_dir, patches_dir, settings_file/settings_dir,
constraints_file, cleanup, variant, jobs, network_isolation, and
min_release_age, and stating it returns None; place the docstring directly after
the def main(...) signature so future maintainers understand the CLI contract
and override rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bbdcde0a-dfa2-4165-9c30-073413d53171
📒 Files selected for processing (1)
src/fromager/__main__.py
Add a new -O/--output-dir option that sets the base directory for sdists-repo, wheels-repo, and work-dir. The individual directory options are hidden but still functional for backwards compatibility. When --output-dir is "." (the default), behavior is identical to before. Closes: python-wheel-build#721 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Justin Larkin <jlarkin@redhat.com> Made-with: Cursor
jlarkin09
force-pushed
the
feat/add-output-dir-cli-option
branch
from
215987c to
7df9620
Compare
1 participant
Add a new -O/--output-dir option that sets the base directory for sdists-repo, wheels-repo, and work-dir. The individual directory options are hidden but still functional for backwards compatibility. When --output-dir is "." (the default), behavior is identical to before.
Simplifies the CLI by consolidating three separate directory options into one. Addresses review feedback from #905.
Closes: #721